feat(tools): add URLReadTool for reading arbitrary URLs - #6834

lib/crewai-tools/src/crewai_tools/rag/loaders/pdf_loader.py (1)

51-54: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Consider bounding this download with safe_get_bounded.

_fetch_from_url reads response.content with no size cap. The body now stays in memory for the whole extraction, so a large remote PDF costs RAM instead of a temp file. This PR adds safe_get_bounded in lib/crewai-tools/src/crewai_tools/security/safe_requests.py, which abandons the stream once the body crosses a limit. Reusing it here gives the loader the same protection the new URLReadTool has.

This is not a regression: the previous temp-file version also read the full body. Treat it as a follow-up if the test churn is unwelcome, because test_load_pdf_from_url and its siblings mock requests.get and read content from the mock.

♻️ Sketch

         try:
-            response = safe_get(url, headers=headers, timeout=30)
-            response.raise_for_status()
-            return response.content
+            body, _content_type, _final_url = safe_get_bounded(
+                url, max_bytes=_MAX_PDF_BYTES, timeout=30, headers=headers
+            )
+            return body
         except Exception as e:
             raise ValueError(f"Failed to download PDF from {url}: {e!s}") from e

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/crewai-tools/src/crewai_tools/rag/loaders/pdf_loader.py` around lines 51
- 54, Update _fetch_from_url to use safe_get_bounded instead of safe_get when
downloading the PDF, preserving the existing headers, timeout, status
validation, and content return behavior while enforcing the shared response-size
limit from safe_requests.

lib/crewai-tools/tests/rag/test_pdf_loader.py (1)

37-45: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Patch safe_get instead of requests.get to remove the DNS dependency.

PDFLoader._fetch_from_url calls safe_get, and safe_get calls validate_url before it issues the request. validate_url resolves the hostname. These tests therefore need working DNS resolution for example.com, and they fail in a network-isolated runner for a reason unrelated to the PDF loader. Patching crewai_tools.rag.loaders.pdf_loader.safe_get removes the dependency and stops the tests from asserting on safe_get internals.

The same change applies to test_load_pdf_from_url_leaves_no_temp_file (Lines 55-73), test_load_pdf_from_url_with_custom_headers (Lines 75-89), and test_load_pdf_url_download_error (Lines 91-94).

♻️ Proposed change for `test_load_pdf_from_url`

     def test_load_pdf_from_url(self):
-        with patch("requests.get") as mock_get:
+        with patch(
+            "crewai_tools.rag.loaders.pdf_loader.safe_get"
+        ) as mock_get:
             mock_get.return_value = Mock(
                 content=build_pdf("Content from URL"),
                 raise_for_status=Mock(),
                 status_code=200,
                 headers={},
             )
             result = PDFLoader().load(SourceContent("https://example.com/report.pdf"))

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/crewai-tools/tests/rag/test_pdf_loader.py` around lines 37 - 45, Update
the URL-based PDF loader tests—test_load_pdf_from_url,
test_load_pdf_from_url_leaves_no_temp_file,
test_load_pdf_from_url_with_custom_headers, and
test_load_pdf_url_download_error—to patch
crewai_tools.rag.loaders.pdf_loader.safe_get instead of requests.get,
configuring the mocked safe_get response or exception as needed while preserving
each test’s existing assertions.

lib/crewai-tools/tests/url_read_tool_test.py (1)

144-159: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Exercise the extension fallback through run instead of _resolve_kind.

These two tests assert on the private method _resolve_kind. The same cases are observable through tool.run with a patched safe_get_bounded, as the surrounding tests do. Behavior-focused tests survive a refactor of the classification internals.

Based on coding guidelines: "Write unit tests for new functionality that focus on behavior rather than implementation details."

♻️ Example replacement

-def test_octet_stream_falls_back_to_url_extension():
-    tool = URLReadTool()
-    assert (
-        tool._resolve_kind("application/octet-stream", "https://example.com/a/b.pdf")
-        == "pdf"
-    )
-    assert tool._resolve_kind("", "https://example.com/a/b.csv") == "text"
-    assert tool._resolve_kind("", "https://example.com/a/b.bin") is None
+def test_octet_stream_csv_falls_back_to_url_extension():
+    tool = URLReadTool()
+    with patch(f"{TOOL_MODULE}.safe_get_bounded") as fetch:
+        fetch.return_value = fetch_result(
+            b"a,b\n1,2\n", "application/octet-stream", "https://example.com/a/b.csv"
+        )
+        assert tool.run(url="https://example.com/a/b.csv") == "a,b\n1,2\n"
+
+
+def test_octet_stream_unknown_extension_is_rejected():
+    tool = URLReadTool()
+    with patch(f"{TOOL_MODULE}.safe_get_bounded") as fetch:
+        fetch.return_value = fetch_result(
+            b"\x00\x01", "application/octet-stream", "https://example.com/a/b.bin"
+        )
+        assert "Unsupported content type" in tool.run(url="https://example.com/a/b.bin")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/crewai-tools/tests/url_read_tool_test.py` around lines 144 - 159, Update
test_octet_stream_falls_back_to_url_extension and
test_query_string_does_not_break_extension_fallback to exercise the extension
fallback through URLReadTool.run rather than the private _resolve_kind method.
Patch safe_get_bounded as in the surrounding tests, provide responses for the
PDF, CSV, and binary URLs, and assert the observable run results while
preserving the existing expected classifications.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

lib/crewai-tools/src/crewai_tools/tools/url_read_tool/url_read_tool.py:342

• BaseTool.run() only validates args_schema when called with keyword args. In _run(), start_line = start_line or 1 / line_count = line_count or None means positional calls like tool.run(url, 0, 0) or tool.run(url, -5, -5) bypass schema validation and get silently coerced (or return an empty window) instead of being rejected as the schema/doc/tests imply. Add explicit validation/coercion here so invalid values are refused regardless of how the tool is called.

        """Fetch a URL and return its content, or a window of it, as text."""
        start_line = start_line or 1
        line_count = line_count or None

lib/crewai-tools/src/crewai_tools/rag/loaders/pdf_loader.py:65

• kwargs.get("max_bytes", DEFAULT_MAX_PDF_BYTES) will pass through an explicit max_bytes=None (or other non-int) from a caller, which then causes a TypeError inside safe_get_bounded when comparing total > max_bytes. Normalize/validate max_bytes before calling safe_get_bounded so None reliably falls back to the default ceiling.

            body, _content_type, _final_url = safe_get_bounded(
                url,
                max_bytes=kwargs.get("max_bytes", DEFAULT_MAX_PDF_BYTES),
                headers=headers,
                timeout=30,
            )

lib/crewai-tools/src/crewai_tools/tools/url_read_tool/url_read_tool.py:346

• start_line = start_line or 1 / line_count = line_count or None silently normalizes falsy values (e.g., 0) instead of rejecting them. Because BaseTool.run skips args_schema validation when positional args are used, callers can bypass the ge=1 constraint and get surprising behavior (e.g., line_count=0 reads the whole content). Add explicit runtime validation here and only default when the value is actually None.

        start_line = start_line or 1
        line_count = line_count or None

lib/crewai-tools/src/crewai_tools/tools/url_read_tool/url_read_tool.py:347

• start_line = start_line or 1 / line_count = line_count or None conflates 0 with None and doesn’t guard negative values (e.g. start_line=-5 stays negative). Because BaseTool.run only validates args_schema when called with keyword args, positional callers can bypass the ge=1 constraints and get surprising behavior (like line_count=0 returning the full content). Consider explicit validation/normalization here so runtime behavior matches the schema constraints even when validation is bypassed.

        start_line = start_line or 1
        line_count = line_count or None